Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enhance CardDAV query report to return a selective list of vcard prop… #903

Merged
merged 1 commit into from
Nov 17, 2016
Merged

Enhance CardDAV query report to return a selective list of vcard prop… #903

merged 1 commit into from
Nov 17, 2016

Conversation

DeepDiver1975
Copy link
Member

…erties - fixes #889

@evert

@DeepDiver1975
Copy link
Member Author

is that related to my changes?

1) Sabre\DAV\Auth\Backend\AbstractBasicTest::testRequireAuth
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Basic realm="writing unittests on a saturday night"'
+'Basic realm="writing unittests on a saturday night", charset="UTF-8"'

/home/deepdiver/Development/ownCloud/sabre-dav/tests/Sabre/DAV/Auth/Backend/AbstractBasicTest.php:69

@staabm
Copy link
Member

staabm commented Nov 15, 2016

I somehow remember a PR which was merged lately to add the charset for the basic auth backend. I guess this is not related to your PR

@staabm
Copy link
Member

staabm commented Nov 15, 2016 •
edited
Loading

Found it:
sabre-io/http#70

Not sure why it fails here, maybe a dependency version problem.
Update: hmm .. The expected value is wrong after the related PR landed

evert
evert requested changes Nov 16, 2016
View reviewed changes
Copy link
Member

@evert evert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the issue with sabre/http. This was indeed an update in sabre/http that happened earlier and now caused sabre/dav to fail.

There is still a small CS problem though. Just run ./bin/sabre-cs-fixer fix lib/Sabre/CardDAV/Plugin.php

lib/CardDAV/Plugin.php
* @return string
*/
protected function convertVCard($data, $target) {
protected function convertVCard($data, $target, $propertiesFilter = null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this get can array typehint?

@DeepDiver1975 DeepDiver1975 force-pushed the vcard-filter-for-addressbook-query branch from ed21376 to ea633dc Compare November 16, 2016 10:11
@DeepDiver1975 DeepDiver1975 force-pushed the vcard-filter-for-addressbook-query branch from ea633dc to c1bb682 Compare November 16, 2016 10:14
@DeepDiver1975
Copy link
Member Author

@evert done

@evert evert merged commit 52bec82 into sabre-io:master Nov 17, 2016
@evert
Copy link
Member

evert commented Nov 17, 2016

Awesome feature @DeepDiver1975

@DeepDiver1975 DeepDiver1975 deleted the vcard-filter-for-addressbook-query branch November 17, 2016 07:49
@DeepDiver1975
Copy link
Member Author

My pleasure. What's your view on backporting this?

@evert
Copy link
Member

evert commented Nov 24, 2016

Totally up for a backport! Doesn't seem like this would cause any sort of BC breaks. Happy to all the way back to 3.0 (which is the lowest supported version).

@DeepDiver1975
Copy link
Member Author

DeepDiver1975 commented Nov 25, 2016 •
edited
Loading

here we go:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evert evert evert approved these changes

Assignees
No one assigned
Labels
component-carddav enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for property filters in addressbook-query REPORT
3 participants
@DeepDiver1975 @staabm @evert